-
-
Notifications
You must be signed in to change notification settings - Fork 629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Justin808/webpacker integration #822
Conversation
3cea6dc
to
791a68b
Compare
const { env, paths, publicPath } = webpackConfigLoader(resolve('..', 'config', 'webpack')); | ||
const webpackConfigLoader = require('react-on-rails/node_package/lib/webpackConfigLoader').default; | ||
const configPath = resolve('..', 'config', 'webpack'); | ||
const { env, paths, publicPath } = webpackConfigLoader(configPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexfedoseev @robwise Does this look OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justin808 What are your concerns here? JS looks ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexfedoseev are you okay with making everyone modify their webpack configs to import env, paths, and publicPath from this react-on-rails lib for webpackConfigLoader
instead of setting themselves manually as normal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to change this around a bit as there seems to be no reason to have the webpack config helper go through babel. Thus, I can move the file so that the
require('react-on-rails/node_package/lib/webpackConfigLoader')
is simpler, like this:
require('react-on-rails/webpackConfigLoader')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexfedoseev are you okay with making everyone modify their webpack configs to import env, paths, and publicPath from this react-on-rails lib for webpackConfigLoader instead of setting themselves manually as normal?
Sounds like unnecessary and leaky layer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexfedoseev and @robwise I have no idea on what you're referring to. The only reason to modify the webpack configs is so that the webpacker_lite view helper will be able to go the public output directory and thus bypass the asset pipeline.
CC: @kaizencodes be sure to check out any commits here. |
791a68b
to
e9d4cce
Compare
1 similar comment
aa621eb
to
e4810b6
Compare
@kaizencodes I've released the beta: Gem: 7.1.0.beta.2 CC: @dzirtusss @udovenko @Judahmeek @robwise You shouldn’t need to change anything at all besides the versions. I want to know if this switch to webpacker for the generators breaks existing apps first. Next step will be a README for the migration to user webpacker_lite and to avoid the asset pipeline. |
- add alias for gen examples
* Any version mismatch of gem and node package throws an error. * 6.9 introduced code that would parse the props into a Hash unnecessarily. Any customers with significant String props took a performance hit from this on page rendering. * Fix is to only put props inside of the script tag, and not any meta data, like the dom id and name of the component. This avoids unnecessary parsing.
* No need to use babel on this file * Add another .eslintrc to avoid error on missing require * Configured package.json to export the file webpackConfigLoader.js
* New default dir reflects generators
eb84ec5
to
95765e3
Compare
I grabbed @kaizencodes latest commit on #811 and rebased my branch on master. PENDING:
|
This should result in no pending changes after tests run locally.
* Removed unnecessary rake task for running specs * Fixed inconsistency with using "-bundle" only some of the time. * Removed unused node_modules value in paths.yml * Upgraded to webpacker_lite 1.0.0
1 similar comment
* Added CHANGELOG.md entry * New default for symlink_non_digested_assets_regex is nil for webpacker_lite default. Previously: symlink_non_digested_assets_regex: /\.(png|jpg|jpeg|gif|tiff|woff|ttf|eot|svg|map)/,
This builds on #811.
This change is